Add support for embedded variables in headers#349
Conversation
|
This looks like a duplicate of #332 Did you consider storing |
Sorry about that! I should have searched for other PRs before opening mine. As far as I'm concerned, it doesn't really matter which PR gets merged. I'll take a look at your code.
Yes, I did - but it's an ugly and clumsy hack. All tools (GitHub CLI, SDKs, etc.) standardize on But what's worse is the very annoying and confusing default behavior. I've never seen a tool that claims to support "variables", but only resolves a variable if it's the only content of the string. This is not documented and extremely counterintuitive. In fact, it's so confusing that I didn't even know what to search for and ended up debugging the code to figure out why I was getting 401 or 403 no matter what I tried. At that point, implementing proper variable support, including tests, only took a quarter of an hour, and I didn't bother looking for issues. So if you decide to reject the PR, please at least document the weird notion of the variable directly where variables are mentioned, to save others searching and re-implementing the same. |
| for env_var_name in re.findall(r"\$([A-z][A-z0-9_]*)", value): | ||
| var_value = os.environ.get(env_var_name) |
There was a problem hiding this comment.
| for env_var_name in re.findall(r"\$([A-z][A-z0-9_]*)", value): | |
| var_value = os.environ.get(env_var_name) | |
| for env_var_name in re.findall(r"\$([A-z][A-z0-9_]*|{[A-z][A-z0-9_]*})", value): | |
| var_value = os.environ.get(env_var_name.removeprefix("{").removesuffix("}")) |
... to support ${VARIABLES} if desired.
DamianCzajkowski
force-pushed
the
main
branch
from
e74ba77 to
e8458ec
Compare
2 participants
Hey,
Thanks for this awesome generator!
Unfortunately, I can't use it to fetch GitHub's own schemas because they require
Authorizationheaders in the form ofBearer token. Currently, the generator only supports variable expansion if it's the only element in the header value. However, I can't hardcode the token in mypyproject.tomlto work around this limitation.So I added support for embedded and multiple variables (not that I need multiple variables, but you basically get that for free with my approach, so why not...). There may be situations where it won't work like shell, but I would prefer simplicity over handling all possible corner cases.
Hope this helps you and others!